Skip to content

feat(deep_causality_tensor): Improved error handling#436

Merged
marvin-hansen merged 7 commits intodeepcausality-rs:mainfrom
marvin-hansen:main
Dec 31, 2025
Merged

feat(deep_causality_tensor): Improved error handling#436
marvin-hansen merged 7 commits intodeepcausality-rs:mainfrom
marvin-hansen:main

Conversation

@marvin-hansen
Copy link
Copy Markdown
Member

@marvin-hansen marvin-hansen commented Dec 31, 2025

User description

Describe your changes

Improved error handling

Issue ticket number and link

Code checklist before requesting a review

  • I have signed the DCO?
  • All tests are passing when running make test?
  • No errors or security vulnerabilities are reported by make check?

For details on make, please see BUILD.md

Note: The CI runs all of the above and fixing things before they hit CI speeds
up the review and merge process. Thank you.


PR Type

Enhancement


Description

  • Replace panic-prone expect() calls with proper Result error handling

  • Use ok_or_else() to convert Option to Result with custom error types

  • Return CausalTensorError::EinSumError for invalid tensor indices

  • Improve error propagation in einsum contraction operations


Diagram Walkthrough

flowchart LR
  A["expect panic calls"] -->|"Replace with"| B["ok_or_else Result"]
  B -->|"Return"| C["CausalTensorError"]
  C -->|"Propagate via"| D["? operator"]
Loading

File Walkthrough

Relevant files
Error handling
ein_sum_impl.rs
Replace expect panics with Result error handling                 

deep_causality_tensor/src/types/cpu_tensor/ops/tensor_ein_sum/ein_sum_impl.rs

  • Replace two expect() calls with ok_or_else() for proper error handling
  • Convert Option values to Result type with
    CausalTensorError::EinSumError
  • Use EinSumValidationError::InvalidAxesSpecification for index out of
    bounds errors
  • Enable error propagation via ? operator instead of panicking
+17/-7   

Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Dec 31, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Internal error detail: The new error messages include internal implementation detail ("lhs/rhs index out of
bounds in contraction"), which may be surfaced to end-users depending on how
CausalTensorError::EinSumError is presented.

Referred Code
// Get values and accumulate - use Result for proper error handling
let lhs_val = lhs.get(&lhs_index).ok_or_else(|| {
    CausalTensorError::EinSumError(
        EinSumValidationError::InvalidAxesSpecification {
            message: "Internal error: lhs index out of bounds in contraction"
                .to_string(),
        },
    )
})?;
let rhs_val = rhs.get(&rhs_index).ok_or_else(|| {
    CausalTensorError::EinSumError(
        EinSumValidationError::InvalidAxesSpecification {
            message: "Internal error: rhs index out of bounds in contraction"
                .to_string(),
        },
    )
})?;

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Dec 31, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use a more appropriate error type

Replace the EinSumValidationError::InvalidAxesSpecification error with
CausalTensorError::InvalidOperation for index-out-of-bounds errors to more
accurately reflect that an internal operation has failed.

deep_causality_tensor/src/types/cpu_tensor/ops/tensor_ein_sum/ein_sum_impl.rs [285-301]

 // Get values and accumulate - use Result for proper error handling
 let lhs_val = lhs.get(&lhs_index).ok_or_else(|| {
-    CausalTensorError::EinSumError(
-        EinSumValidationError::InvalidAxesSpecification {
-            message: "Internal error: lhs index out of bounds in contraction"
-                .to_string(),
-        },
+    CausalTensorError::InvalidOperation(
+        "Internal error: lhs index out of bounds in contraction".to_string(),
     )
 })?;
 let rhs_val = rhs.get(&rhs_index).ok_or_else(|| {
-    CausalTensorError::EinSumError(
-        EinSumValidationError::InvalidAxesSpecification {
-            message: "Internal error: rhs index out of bounds in contraction"
-                .to_string(),
-        },
+    CausalTensorError::InvalidOperation(
+        "Internal error: rhs index out of bounds in contraction".to_string(),
     )
 })?;
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that InvalidAxesSpecification is semantically incorrect for an index-out-of-bounds error and proposes using the more appropriate InvalidOperation error variant, which improves the clarity and correctness of error handling.

Medium
General
Add index and shape to message

Improve error diagnostics by including the actual index and tensor shape in the
error message for out-of-bounds access.

deep_causality_tensor/src/types/cpu_tensor/ops/tensor_ein_sum/ein_sum_impl.rs [289-290]

-"Internal error: lhs index out of bounds in contraction".to_string()
+format!(
+    "lhs index {:?} out of bounds for shape {:?}",
+    &lhs_index, lhs.shape()
+)

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: This suggestion correctly proposes adding more context to the error message, which would significantly improve debuggability. However, it overlaps with suggestion 1, which addresses the more fundamental issue of using the correct error type.

Low
  • Update

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 31, 2025

Codecov Report

❌ Patch coverage is 12.50000% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.91%. Comparing base (be92279) to head (831f15b).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...ypes/cpu_tensor/ops/tensor_ein_sum/ein_sum_impl.rs 12.50% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #436      +/-   ##
==========================================
+ Coverage   86.32%   92.91%   +6.58%     
==========================================
  Files         777      777              
  Lines       31698    31708      +10     
==========================================
+ Hits        27362    29460    +2098     
+ Misses       4336     2248    -2088     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
@marvin-hansen marvin-hansen merged commit 9d36dcc into deepcausality-rs:main Dec 31, 2025
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant